fix(todo): preserve compatibility outside active plans - #8334
Conversation
E2E verification reportExecuted Observed behavior: an ordinary prompt completed normally; the daemon transcript contained the turn; the model received the active-plan-only Agent association guidance and did not receive the obsolete unconditional “create a Todo first” instruction. Evidence: ordinary-task.png. The local machine-readable result and daemon/model logs are under The changed-file lint, Core typecheck, Core regressions, Web Shell plan regression, and the real E2E completed. The root build and Web Shell typecheck remain blocked by unrelated main-branch Ink selection API and |
|
Thanks for the follow-up! Template — complete ✓ (all sections present, bilingual body, linked issue). Problem — observed, not theoretical. #8328 documents concrete edges exposed by the #7580 Session Workflow merge: the system prompt nudged creating a Todo before every top-level Agent call, a distinct all-completed snapshot reused the finished plan's Direction — aligned. This narrows the Todo contract back to "opt-in to an existing active plan" and preserves ordinary-session behavior; it does not expand scope or expose Size — tiny: 7 production-logic lines across 3 files ( Approach — scope feels right; each of the three edits maps 1:1 to an issue point and carries its own test, with no drive-by churn. I checked the Risk — no elevated signals; none of the changed files match the high-revert paths. Moving on to code review. 🔍 中文说明感谢这个 follow-up! 模板 — 完整 ✓(各节齐全、双语正文、关联 issue)。 问题 — 已观测,非理论。#8328 记录了 #7580 Session Workflow 合并后暴露的具体边界:系统 prompt 会引导在每个顶层 Agent 调用前创建 Todo;独立的全 completed 快照会复用已完成计划的 方向 — 对齐。把 Todo 契约收回到"按需加入已有 active 计划",保留普通 session 行为;没有扩大范围,也没有向 subagent 暴露 规模 — 很小:3 个文件共 7 行生产逻辑( 方案 — 范围合理;三处改动与 issue 各点一一对应,各自带测试,无顺手杂改。我特别核对了 风险 — 无升级信号;改动文件均未命中高回滚路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code reviewI wrote my independent proposal first: for the No blockers. What I verified:
Tests are load-bearing: the new "distinct all-completed snapshot" case and the unknown-dependency case both fail on the old code. Conventions clean (ESM, no Test evidenceQuoting the PR's own CI on
The deterministic claims (planId lifecycle, dependency blocking, prompt text) are pinned by the green unit suite. The one thing the suite does not pin is the end-to-end behavioural effect of the prompt narrowing — that an ordinary task actually completes without spawning a Todo plan — which rests on the author's macOS-only E2E. Sandboxed verification would settle it: 中文说明代码审查我先独立写了方案:对 无阻塞项。核对要点:
测试是承重的:新增的"独立全 completed 快照"与"未知依赖"两个用例在旧代码上都会失败。约定干净(ESM、无 测试证据引用 PR 自身在 确定性结论(planId 生命周期、依赖阻塞、prompt 文本)由绿色单测固定。唯一未被单测固定的是 prompt 收窄的端到端行为效果——普通任务是否真的不再生成 Todo 计划——目前依赖作者仅在 macOS 上的 E2E。沙箱验证可定论: — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 4/5 — clean, minimal, well-tested fix; the only reservation is that the prompt's end-to-end behavioural effect isn't pinned by the suite. Stepping back: this is exactly the kind of follow-up I want to see after a big feature merge. #7580 widened the Todo contract for everyone; this PR quietly puts the ordinary-session behavior back the way it was, fixes two real lifecycle/render edges, and ships a test for each. My independent proposal and the diff converged — I didn't find a simpler path it missed. The change is easy to maintain: three small, self-explanatory edits, no new abstraction, reusing the stdlib deep-equal the codebase already leans on. CI is green on the commit I reviewed. The half-point I'm holding back is honest, not blocking — the unit suite proves the mechanism but not that a model will actually stop creating plans for ordinary tasks; that's the author's macOS E2E, and a Approving, pinned to the reviewed commit. 中文说明置信度:4/5 — 干净、精简、测试充分的修复;唯一保留是 prompt 的端到端行为效果未被测试套件固定。 退一步看:这正是大 feature 合并后我希望看到的 follow-up。#7580 把 Todo 契约对所有人放宽;本 PR 悄悄把普通 session 的行为恢复原状,修掉两个真实的生命周期/渲染边界,并各配一个测试。我的独立方案与 diff 收敛——没找到更简的路径。 改动易于维护:三处小而自明、无新抽象、复用仓库已在用的标准库深比较。我所审查的提交上 CI 为绿。扣掉的半分是诚实而非阻塞——单测证明了机制,但没证明模型真的会停止为普通任务创建计划;那是作者仅在 macOS 的 E2E, 批准,固定到所审查的提交。 — Qwen Code · qwen3.8-max-preview Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 1 render-shaping file:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM!✅
— qwen3.8-max-preview via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review
Independent verification report (local real stack)Verified this PR locally on a real stack: a real Environment: macOS arm64, Node v24.18.1, PR head 1. Unit tests (before/after)
Every new test fails on 2. Plan identity for all-completed snapshots (
|
| Node | before (main) | after (PR) |
|---|---|---|
mig-3 — depends on pruned mig-0 |
blocked |
ready |
mig-2 — depends on completed mig-1 |
ready |
ready (known-dep semantics unchanged) |
Before (main frontend) — mig-3 incorrectly shown as Blocked:
After (PR frontend) — same data, mig-3 shown as Ready:
4. Prompt contract (prompts.ts)
Asserted on the actual request bodies received by the model server: all 9 requests issued by the PR daemon contain the new opt-in wording ("When an active Todo plan covers work delegated through top-level Agent calls…") and none contain the old "create the relevant todo first" wording; the base daemon's 9 requests show exactly the inverse. The deployed system prompt matches the PR's contract end-to-end, not just in the snapshot test.
Notes for the record
- The PR body's test-plan step 3 references
.qwen/e2e-tests/issue-8328/run-compatibility-e2e.ts, which is neither part of this diff nor present on the branch (local artifact of the author). The verification above covers that scenario independently — an ordinary prompt completes with no todo plan. - A
Set model failed: POST /session/:id/model 500toast appears at session start in this rig on both stacks (the mock provider doesn't support model switching) — environment artifact, unrelated to this PR.
Verification verdict: the change is minimal, each of the three edges is fixed, and before/after runs show no regression in the neighboring behavior. Good to merge from my side.
中文版本
独立验证报告(本地真实栈)
在本地真实栈上验证了本 PR:真实 qwen serve daemon(从 PR 源码启动)+ 本地确定性 OpenAI 兼容模型服务 + headless Chromium 驱动的 web-shell 界面,并与 main 做了完整的修复前/后对照。三处修复全部端到端验证通过,修复边界之外的行为无变化。
环境:macOS arm64、Node v24.18.1、PR head 7837ff9(merge-base 893a970)、隔离 QWEN_HOME、daemon 走 scripts/dev.js serve、web-shell 走 Vite dev、Playwright headless Chromium。after 栈跑 PR 源码;before 栈用同一棵树仅把三个改动的源码文件回退到 origin/main,配独立 daemon——同一 mock 模型、同一脚本化对话,保证唯一变量就是本 PR 的 diff。
1. 单元测试(前后对照)
| 测试套件 | PR head | PR 测试跑在 main 源码上 |
|---|---|---|
core prompts.test.ts + todoWrite.test.ts |
136 通过 | prompts.test.ts 17 失败(snapshot + 新断言);todoWrite.test.ts 1 失败:should start a new plan for a distinct all-completed snapshot |
web-shell PlanExecutionView.test.tsx |
22 通过 | 1 失败:does not block a todo on an unknown dependency |
每个新增测试在 main 源码上都失败、打上 PR 后通过,说明测试确实锁定了修复。
2. 全 completed 快照的计划身份(todoWrite.ts)
用确定性模型脚本走完整生命周期(真实 daemon + web-shell),判据为持久化 todo 文件(QWEN_HOME/todos/<sessionId>.json):
PR 栈(修复后):普通问答不产生 todo 计划;建计划得 planId=a5757155-…;全部完成后 planId 不变;提交不同的全 completed 快照 → 新 planId=d2cc2364-…(即本修复);重复提交相同快照 → planId 不变(无身份抖动)。
Base 栈(修复前,同一脚本):不同的全 completed 快照仍沿用旧 planId=b5f72308-… —— 在 main 上复现了 bug。
相同快照重复提交在两套栈上都保持 planId,说明 isDeepStrictEqual 守卫只对真正不同的快照发新身份。
3. 历史数据中的未知依赖(PlanExecutionView.tsx)
写入时校验已经拒绝未知 blockedBy 引用,此状态只能来自历史 transcript。手工构造了一个历史会话(projects/<project>/chats/<uuid>.jsonl),其中持久化的 todo_write 结果里 mig-3 的 blockedBy: ["mig-0"] 指向已被清理的 mig-0。同一份数据分别在两个前端加载并打开计划面板,读取节点 data-status:
| 节点 | 修复前(main) | 修复后(PR) |
|---|---|---|
mig-3 —— 依赖已清理的 mig-0 |
blocked |
ready |
mig-2 —— 依赖已完成的 mig-1 |
ready |
ready(已知依赖语义不变) |
截图见上方英文部分:before 图中 mig-3 被误标为 Blocked(黄框),after 图中同一数据显示 Ready。
4. Prompt 契约(prompts.ts)
直接断言模型服务收到的请求体:PR daemon 发出的 9 个请求全部包含新的按需关联措辞("When an active Todo plan covers work delegated through top-level Agent calls…"),无一包含旧的 "create the relevant todo first" 措辞;base daemon 的 9 个请求正好相反。部署态的 system prompt 与 PR 声称的契约端到端一致,而不仅是 snapshot 测试层面。
备注
- PR 描述测试计划第 3 步引用的
.qwen/e2e-tests/issue-8328/run-compatibility-e2e.ts既不在本 diff 中、分支上也不存在(作者本地产物)。上面的验证已独立覆盖该场景——普通 prompt 完成且不产生 todo 计划。 - 会话启动时的
Set model failed: POST /session/:id/model 500toast 在两套栈上都出现(mock provider 不支持切换模型),属环境产物,与本 PR 无关。
验证结论:改动最小化,三处边界各自修复,前后对照未见邻近行为回归。我这边认为可以合并。
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 23 passed · 0 failed · 23 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:23 通过 · 0 失败 · 23 总计 Verification reportPR #8334 Deep Verification —
|
| scenario | HEAD | BASE control | note |
|---|---|---|---|
| S1 create from empty | new | new | ordinary |
| S2 active plan revised (in_progress→completed) | same | same | ordinary |
| S3 new plan after previous completed | new | new | ordinary |
| S4 repeated terminal snapshot (deep-equal) | same | same | no-over-correction pin |
| S5 distinct all-completed snapshot (same id, different content) | new | same | the fix / the bug |
| S6 distinct all-completed + added completed item | new | same | the fix / the bug |
| S7 clear list | display same / disk none | display same / disk none | ordinary |
| T1 reorder an all-completed list | new | same | boundary observation |
T2 add empty blockedBy: [] to an all-completed item |
new | same | boundary observation |
Result: 2/2 discriminating cells (S5, S6) flip same → new; 5/5 ordinary/invariant cells are byte-identical across arms; S4 stays same on head (the fix does not mint a new id for an identical re-write). Invariants self-asserted by the harness on both arms (INVARIANTS_OK). This is the load-bearing proof: the base build conflates two distinct completed plans under one id; the head build separates them while leaving the ordinary flow untouched.
Vacuity / mutation matrix. Each new test was run against HEAD (green control) and against a source tree with only its own hunk reverted (harness/mutation-matrix.sh). Witness: 02-mutation-matrix.png.
| arm | suite | result | discriminating test |
|---|---|---|---|
| HEAD (control) | todoWrite.test.ts |
44/44 pass | — |
| HEAD (control) | PlanExecutionView.test.tsx |
22/22 pass | — |
Mutant A: revert isDeepStrictEqual hunk |
todoWrite.test.ts |
1 failed | 43 passed | ✗ "should start a new plan for a distinct all-completed snapshot" → expected 'finished-plan' not to be 'finished-plan' |
Mutant B: drop todosById.has(id) && guard |
PlanExecutionView.test.tsx |
1 failed | 21 passed | ✗ "does not block a todo on an unknown dependency" → expected { status: 'blocked' } to deeply equal { status: 'ready' } |
Both new tests fail the intended behavioural assertion on base (not an import/compile/fixture break) ⇒ non-vacuous. Mutant B's run also keeps the pre-existing "uses live execution state before todo and dependency state" test green (a known in-progress dependency still renders blocked) ⇒ the UI fix does not over-correct. The repeated-terminal-snapshot test ('Still done'→'Done' fixture edit) passes on both arms, as the A/B predicts.
Secondary claim — prompt contract. prompts.ts now reads "When an active Todo plan covers work delegated through top-level Agent calls, pass the matching Todo ID … Do not create a Todo solely to wrap a delegation…". Pinned by prompts.test.ts (new toContain / not.toContain) and the snapshot (the line changed across all prompt variants, nothing else). prompts.test.ts is green (92/92). The old wording survives only as the not.toContain assertion — no source/snapshot still expects it. The behavioural effect of this wording (model creates fewer wrapper-todos) is model-mediated and not deterministically A/B-able in this environment; the prompt-contract test is the deterministic proxy.
Corrections
- The PR description's "Risk & Scope" note is stale (favourable). It states "Full root build and Web Shell typecheck are currently blocked by unrelated main-branch Ink selection API and
DaemonSessionActions.removeMidTurnMessagemismatches." At the verified merge commit, both affected packages typecheck clean:packages/coretsc --noEmit→ 0 errors;packages/web-shelltsc -p tsconfig.json --noEmit→ 0 errors. The web-shell gate was proven live (a plantedconst x: number = "…"producederror TS2322atPlanExecutionView.tsx(741,7), exit 2, then removed). The blocker the author recorded no longer reproduces atHEAD^1..HEAD(most likely resolved by the mergedmain/ the v0.21.3 base). This is a correction to the description, not a request to change code.
Findings
- (Informational / non-blocking)
isDeepStrictEqualis order- and field-sensitive, so two semantically-identical completed snapshots can get distinct plan ids. Measured in the A/B: T1 (reordering an all-completed list) and T2 (adding an emptyblockedBy: []) both flipsame → newon head. Assessment: this only fires when a model re-writes an already all-completed list with reordered items or a toggled emptyblockedBy, which is outside the ordinary flow; the consequence is a new plan id on a completed-only snapshot —unfinishedTodosis empty, so no active-todo reminder is set and there is no active-task side effect; downstream consumers (PlanEmitter, web-shelltodos.ts,transcript-replay) treatplanIdas an opaque grouping key, so a fresh key for a genuinely distinct snapshot is the PR's stated intent. Not a defect; recorded so the author knows the equality is structural (order + exact field set), in case a future flow re-emits completed lists in a different order and expects id stability. No fix suggested — behaviour is benign and intent-aligned.
Not covered
- Reviewer Test Plan step 3 (E2E) could not be run.
npx tsx .qwen/e2e-tests/issue-8328/run-compatibility-e2e.tsis referenced in the PR body but.qwen/is git-ignored, so the harness is absent from the verification checkout (git check-ignoreconfirms). Its target behaviour ("ordinary prompt completes without a forced Todo plan") is model-mediated; the deterministic proxies here are the real-fs A/B ordinary-lifecycle cells S1–S4 (preserved identically) and the prompt-contract test. This reproduces the handling (plan-identity decision and prompt wording), not a live model-driven trigger. - Per-commit attribution.
$QWEN_VERIFY_CONTEXTlists 3 commits but the checkout is shallow (depth 2);git rev-list HEAD^1..HEAD^2yields only the merge commit7837ff9d. Per the multi-commit rule, per-commit claims were out of reach; the aggregateHEAD^1..HEADdiff (7 files, +63/−21) was verified. - Full workspace suites / repo-wide lint / consumer-package tests not run. Gates run here:
todoWrite.test.ts(44),prompts.test.ts(92),PlanExecutionView.test.tsx(22), plustsc --noEmitforpackages/coreandpackages/web-shell. The fullpackages/coreandpackages/web-shellsuites,npm run lint, and tests ofplanIdconsumers (packages/cliPlanEmitter,packages/web-shell/client/utils/todos.test.ts,packages/acp-bridge) were not run — the PR's own CI covers these, and no consumer relies on the old "unknown dep = blocked" or "distinct completed snapshot reuses id" behaviour (grepped). - Model-behavioural effect of the prompt change (fewer wrapper-todos created during real delegations) — not measurable without a live model; see Secondary claim above.
Methodology
Environment: CI verify lane, node:22-bookworm, Node v22.23.2; merge-ref checkout at depth 2 (HEAD merge commit, HEAD^1 base tip, HEAD^2 verified PR head); npm ci + npm run build pre-run. The central A/B drove the real compiled TodoWriteTool from packages/core/dist against a real temp-dir filesystem (QWEN_RUNTIME_DIR), stubbing only the 3-method Config collaborator; the base arm patched the single changed hunk in the compiled output and restored from a byte-identical backup (confirmed via diff). Vacuity used the project's own vitest with only the relevant source hunk reverted (restored via git checkout; tree confirmed clean throughout). Typecheck gates were proven live with a planted-error control. Raw per-cell logs (harness/head-run.txt, base-run.txt, matrix-*.txt, *-typecheck-head.txt), the harnesses (ab-todowrite.mjs, ab-driver.sh, mutation-matrix.sh, grade.mjs), and grade-output.txt are in this artifact dir; assertions.json is emitted by grade.mjs, which re-executes the A/B driver, the mutation matrix, and the prompt suite and encodes expected base-arm reds as passes (so fail counts only unexpected outcomes). Image evidence: 01-ab-plan-identity-head-vs-base.png, 02-mutation-matrix.png, 03-grader-assertions.png.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.21.4. |






What this PR does
This follow-up preserves ordinary Todo behavior outside Session Workflow views. Agent association is now opt-in to an existing active Todo plan, a distinct all-completed snapshot receives a new plan identity, and unknown historical dependency references no longer make a plan node appear blocked.
Why it's needed
The Session Workflow changes merged in #7580 introduced a wider prompt instruction and exposed lifecycle/UI edges that could affect normal tasks or misrepresent historical plans. The parent Session remains the owner of persisted Todo state; this PR does not expose
todo_writeto subagents or teammates.Reviewer Test Plan
How to verify
cd packages/core && npx vitest run src/core/prompts.test.ts src/tools/todoWrite.test.tsand confirm the prompt contract and distinct completed-plan identity cases.cd packages/web-shell && npx vitest run client/components/messages/PlanExecutionView.test.tsxand confirm an unknown dependency is displayed as ready rather than blocked.npx tsx .qwen/e2e-tests/issue-8328/run-compatibility-e2e.ts. This starts a real daemon, deterministic local model, and production Web Shell; submit an ordinary prompt and confirm it completes without a Todo plan.Evidence (Before & After)
The real daemon/Web Shell run completed an ordinary task and captured the rendered result:
The detailed local report is in
.qwen/e2e-tests/issue-8328/report.md.Tested on
Environment (optional)
macOS arm64, Node.js v22.22.0, Chrome headless, real
qwen serve, local deterministic OpenAI-compatible test server,QWEN_SANDBOX=false.Risk & Scope
DaemonSessionActions.removeMidTurnMessagemismatches; these are recorded in the E2E report.Linked Issues
Fixes #8328
中文说明
本 PR 做了什么
这个 follow-up 保证 Session Workflow 视图之外的普通 Todo 行为保持不变。Agent 关联现在只在已有 active Todo plan 时按需开启;独立的全 completed 快照会获得新的 plan identity;历史数据中的未知依赖不会再把节点误显示为 blocked。
为什么需要
#7580 合并的 Session Workflow 改造扩大了 prompt 指令范围,并暴露了可能影响普通任务或误读历史计划的生命周期/UI 边界。持久化 Todo 仍由父 Session 独占;本 PR 不会向 subagent 或 teammate 暴露
todo_write。Reviewer Test Plan
如何验证
cd packages/core && npx vitest run src/core/prompts.test.ts src/tools/todoWrite.test.ts,确认 prompt 契约和独立全 completed 计划 identity 场景。cd packages/web-shell && npx vitest run client/components/messages/PlanExecutionView.test.tsx,确认未知依赖显示为 ready 而不是 blocked。npx tsx .qwen/e2e-tests/issue-8328/run-compatibility-e2e.ts。该命令启动真实 daemon、本地确定性模型和生产 Web Shell;提交普通 prompt,确认任务完成且没有被强制创建 Todo plan。证据(Before & After)
真实 daemon/Web Shell 运行完成了普通任务并捕获了渲染结果:
详细本地报告位于
.qwen/e2e-tests/issue-8328/report.md。测试环境
macOS arm64、Node.js v22.22.0、Chrome headless、真实
qwen serve、本地确定性 OpenAI-compatible 测试服务、QWEN_SANDBOX=false。风险与范围
DaemonSessionActions.removeMidTurnMessage不匹配阻塞,已记录在 E2E 报告中。